[XENOPROF] Some fixes for xenoprof passive domain support.

Currently, passive domain samples are being assigned to the wrong
kernel functions. This patch fixes this problem. In addition the patch changes the
encoding of domain switch ESCAPE codes (marks used to separate samples
in oprofile  buffers associated with different domains). Instead of
using 2 codes, one  for START and one for END of passive domain
samples, a single escape CODE value is used to indicate a domain switch (no
need for a STOP followed by a START). Finally there some other minor style
fixes.

Signed-off-by: Jose Renato Santos <jsantos@hpl.hp.com>

diff --git a/linux-2.6-xen-sparse/arch/i386/oprofile/xenoprof.c b/linux-2.6-xen-sparse/arch/i386/oprofile/xenoprof.c
index 271f8be89b742ef2fe49e92aa5de3c28563c2981..34f4d9726055cad28d37727354d7650f2d182f05 100644 (file)
--- a/linux-2.6-xen-sparse/arch/i386/oprofile/xenoprof.c
+++ b/linux-2.6-xen-sparse/arch/i386/oprofile/xenoprof.c
@@ -29,6 +29,7 @@
 #include <xen/interface/xen.h>
 #include <xen/interface/xenoprof.h>
 #include <../../../drivers/oprofile/cpu_buffer.h>
+#include <../../../drivers/oprofile/event_buffer.h>
 
 static int xenoprof_start(void);
 static void xenoprof_stop(void);
@@ -151,16 +152,27 @@ static void xenoprof_add_pc(xenoprof_buf_t *buf, int is_passive)
 static void xenoprof_handle_passive(void)
 {
        int i, j;
-
-       for (i = 0; i < pdomains; i++)
+       int flag_domain, flag_switch = 0;
+       
+       for (i = 0; i < pdomains; i++) {
+               flag_domain = 0;
                for (j = 0; j < passive_domains[i].nbuf; j++) {
                        xenoprof_buf_t *buf = p_xenoprof_buf[i][j];
                        if (buf->event_head == buf->event_tail)
                                continue;
-                        oprofile_add_pc(IGNORED_PC, CPU_MODE_PASSIVE_START, passive_domains[i].domain_id);
+                       if (!flag_domain) {
+                               if (!oprofile_add_domain_switch(passive_domains[i].
+                                                               domain_id))
+                                       goto done;
+                               flag_domain = 1;
+                       }
                        xenoprof_add_pc(buf, 1);
-                        oprofile_add_pc(IGNORED_PC, CPU_MODE_PASSIVE_STOP, passive_domains[i].domain_id);
-               }                       
+                       flag_switch = 1;
+               }
+       }
+done:
+       if (flag_switch)
+               oprofile_add_domain_switch(COORDINATOR_DOMAIN);
 }
 
 static irqreturn_t 
@@ -177,6 +189,7 @@ xenoprof_ovf_interrupt(int irq, void * dev_id, struct pt_regs * regs)
 
        if (is_primary && !test_and_set_bit(0, &flag)) {
                xenoprof_handle_passive();
+               smp_mb__before_clear_bit();
                clear_bit(0, &flag);
        }
 

diff --git a/patches/linux-2.6.16.13/xenoprof-generic.patch b/patches/linux-2.6.16.13/xenoprof-generic.patch
index 799794dd3a72663c452d71bea1eeae4bbbd29ee7..66b17ba2f3f2df418c96088418556de752ecfb82 100644 (file)
--- a/patches/linux-2.6.16.13/xenoprof-generic.patch
+++ b/patches/linux-2.6.16.13/xenoprof-generic.patch
@@ -1,6 +1,6 @@
-diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/buffer_sync.c ./drivers/oprofile/buffer_sync.c
---- ../pristine-linux-2.6.16.13/drivers/oprofile/buffer_sync.c 2006-05-03 05:38:44.000000000 +0800
-+++ ./drivers/oprofile/buffer_sync.c   2006-06-27 12:14:53.000000000 +0800
+diff -Naur orig/drivers/oprofile/buffer_sync.c new/drivers/oprofile/buffer_sync.c
+--- orig/drivers/oprofile/buffer_sync.c        2006-05-02 14:38:44.000000000 -0700
++++ new/drivers/oprofile/buffer_sync.c 2006-07-06 18:19:05.000000000 -0700
 @@ -6,6 +6,10 @@
   *
   * @author John Levon <levon@movementarian.org>
@@ -12,7 +12,7 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/buffer_sync.c ./drivers/o
   * This is the core of the buffer management. Each
   * CPU buffer is processed and entered into the
   * global event buffer. Such processing is necessary
-@@ -275,15 +279,30 @@ static void add_cpu_switch(int i)
+@@ -275,15 +279,31 @@
        last_cookie = INVALID_COOKIE;
  }
  
@@ -34,22 +34,23 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/buffer_sync.c ./drivers/o
 +      case CPU_MODE_XEN:
 +              add_event_entry(XEN_ENTER_SWITCH_CODE);
 +              break;
-+        case CPU_MODE_PASSIVE_START:
-+                add_event_entry(PASSIVE_START_CODE);
-+                break;
-+        case CPU_MODE_PASSIVE_STOP:
-+                add_event_entry(PASSIVE_STOP_CODE);
-+                break;
 +      default:
 +              break;
 +      }
  }
 - 
++
++static void add_domain_switch(unsigned long domain_id)
++{
++      add_event_entry(ESCAPE_CODE);
++      add_event_entry(DOMAIN_SWITCH_CODE);
++      add_event_entry(domain_id);
++}
 +
  static void
  add_user_ctx_switch(struct task_struct const * task, unsigned long cookie)
  {
-@@ -348,9 +367,9 @@ static int add_us_sample(struct mm_struc
+@@ -348,9 +368,9 @@
   * for later lookup from userspace.
   */
  static int
@@ -61,7 +62,7 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/buffer_sync.c ./drivers/o
                add_sample_entry(s->eip, s->event);
                return 1;
        } else if (mm) {
-@@ -496,10 +515,11 @@ void sync_buffer(int cpu)
+@@ -496,10 +516,11 @@
        struct mm_struct *mm = NULL;
        struct task_struct * new;
        unsigned long cookie = 0;
@@ -70,34 +71,35 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/buffer_sync.c ./drivers/o
        unsigned int i;
        sync_buffer_state state = sb_buffer_start;
        unsigned long available;
-+      int domain_switch = NO_DOMAIN_SWITCH;
++      int domain_switch = 0;
  
        down(&buffer_sem);
   
-@@ -513,12 +533,19 @@ void sync_buffer(int cpu)
+@@ -512,16 +533,18 @@
+       for (i = 0; i < available; ++i) {
                struct op_sample * s = &cpu_buf->buffer[cpu_buf->tail_pos];
   
-               if (is_code(s->eip)) {
+-              if (is_code(s->eip)) {
 -                      if (s->event <= CPU_IS_KERNEL) {
-+                      if (s->event < CPU_TRACE_BEGIN) {
-                               /* kernel/userspace switch */
+-                              /* kernel/userspace switch */
 -                              in_kernel = s->event;
++              if (is_code(s->eip) && !domain_switch) {
++                      if (s->event <= CPU_MODE_XEN) {
++                              /* xen/kernel/userspace switch */
 +                              cpu_mode = s->event;
                                if (state == sb_buffer_start)
                                        state = sb_sample_start;
 -                              add_kernel_ctx_switch(s->event);
-+
-+                              if (s->event == CPU_MODE_PASSIVE_START)
-+                                      domain_switch = DOMAIN_SWITCH_START_EVENT1;
-+                              else if (s->event == CPU_MODE_PASSIVE_STOP)
-+                                      domain_switch = DOMAIN_SWITCH_STOP_EVENT1;
-+
-+                              if (domain_switch != DOMAIN_SWITCH_START_EVENT2)
-+                                      add_cpu_mode_switch(s->event);
++                              add_cpu_mode_switch(s->event);
                        } else if (s->event == CPU_TRACE_BEGIN) {
                                state = sb_bt_start;
                                add_trace_begin();
-@@ -535,11 +562,20 @@ void sync_buffer(int cpu)
++                      } else if (s->event == CPU_DOMAIN_SWITCH) {
++                                      domain_switch = 1;                              
+                       } else {
+                               struct mm_struct * oldmm = mm;
+ 
+@@ -535,11 +558,16 @@
                                add_user_ctx_switch(new, cookie);
                        }
                } else {
@@ -106,13 +108,9 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/buffer_sync.c ./drivers/o
 -                              if (state == sb_bt_start) {
 -                                      state = sb_bt_ignore;
 -                                      atomic_inc(&oprofile_stats.bt_lost_no_mapping);
-+                      if (domain_switch == DOMAIN_SWITCH_START_EVENT1) {
-+                              add_event_entry(s->event);
-+                              domain_switch = DOMAIN_SWITCH_START_EVENT2;
-+                      } else if (domain_switch == DOMAIN_SWITCH_START_EVENT1) {
-+                              add_sample_entry(s->eip, s->event);
-+                      } else if (domain_switch == DOMAIN_SWITCH_STOP_EVENT1) {
-+                              domain_switch = NO_DOMAIN_SWITCH;
++                      if (domain_switch) {
++                              add_domain_switch(s->eip);
++                              domain_switch = 0;
 +                      } else {
 +                              if (state >= sb_bt_start &&
 +                                  !add_sample(mm, s, cpu_mode)) {
@@ -123,24 +121,9 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/buffer_sync.c ./drivers/o
                                }
                        }
                }
-diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/buffer_sync.h ./drivers/oprofile/buffer_sync.h
---- ../pristine-linux-2.6.16.13/drivers/oprofile/buffer_sync.h 2006-05-03 05:38:44.000000000 +0800
-+++ ./drivers/oprofile/buffer_sync.h   2006-06-27 12:12:09.000000000 +0800
-@@ -9,6 +9,11 @@
- 
- #ifndef OPROFILE_BUFFER_SYNC_H
- #define OPROFILE_BUFFER_SYNC_H
-+
-+#define NO_DOMAIN_SWITCH              -1
-+#define DOMAIN_SWITCH_START_EVENT1    0
-+#define DOMAIN_SWITCH_START_EVENT2    1
-+#define DOMAIN_SWITCH_STOP_EVENT1     2
-  
- /* add the necessary profiling hooks */
- int sync_start(void);
-diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/cpu_buffer.c ./drivers/oprofile/cpu_buffer.c
---- ../pristine-linux-2.6.16.13/drivers/oprofile/cpu_buffer.c  2006-05-03 05:38:44.000000000 +0800
-+++ ./drivers/oprofile/cpu_buffer.c    2006-06-19 22:43:53.000000000 +0800
+diff -Naur orig/drivers/oprofile/cpu_buffer.c new/drivers/oprofile/cpu_buffer.c
+--- orig/drivers/oprofile/cpu_buffer.c 2006-05-02 14:38:44.000000000 -0700
++++ new/drivers/oprofile/cpu_buffer.c  2006-07-06 18:19:05.000000000 -0700
 @@ -6,6 +6,10 @@
   *
   * @author John Levon <levon@movementarian.org>
@@ -152,7 +135,16 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/cpu_buffer.c ./drivers/op
   * Each CPU has a local buffer that stores PC value/event
   * pairs. We also log context switches when we notice them.
   * Eventually each CPU's buffer is processed into the global
-@@ -58,7 +62,7 @@ int alloc_cpu_buffers(void)
+@@ -34,6 +38,8 @@
+ #define DEFAULT_TIMER_EXPIRE (HZ / 10)
+ static int work_enabled;
+ 
++static int32_t current_domain = COORDINATOR_DOMAIN;
++
+ void free_cpu_buffers(void)
+ {
+       int i;
+@@ -58,7 +64,7 @@
                        goto fail;
   
                b->last_task = NULL;
@@ -161,7 +153,7 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/cpu_buffer.c ./drivers/op
                b->tracing = 0;
                b->buffer_size = buffer_size;
                b->tail_pos = 0;
-@@ -114,7 +118,7 @@ void cpu_buffer_reset(struct oprofile_cp
+@@ -114,7 +120,7 @@
         * collected will populate the buffer with proper
         * values to initialize the buffer
         */
@@ -170,7 +162,7 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/cpu_buffer.c ./drivers/op
        cpu_buf->last_task = NULL;
  }
  
-@@ -164,13 +168,13 @@ add_code(struct oprofile_cpu_buffer * bu
+@@ -164,13 +170,13 @@
   * because of the head/tail separation of the writer and reader
   * of the CPU buffer.
   *
@@ -188,7 +180,7 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/cpu_buffer.c ./drivers/op
  {
        struct task_struct * task;
  
-@@ -181,16 +185,14 @@ static int log_sample(struct oprofile_cp
+@@ -181,18 +187,18 @@
                return 0;
        }
  
@@ -207,12 +199,43 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/cpu_buffer.c ./drivers/op
 -
 +      
        /* notice a task switch */
-       if (cpu_buf->last_task != task) {
+-      if (cpu_buf->last_task != task) {
++      /* if not processing other domain samples */
++      if ((cpu_buf->last_task != task) &&
++          (current_domain == COORDINATOR_DOMAIN)) {
                cpu_buf->last_task = task;
-diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/cpu_buffer.h ./drivers/oprofile/cpu_buffer.h
---- ../pristine-linux-2.6.16.13/drivers/oprofile/cpu_buffer.h  2006-05-03 05:38:44.000000000 +0800
-+++ ./drivers/oprofile/cpu_buffer.h    2006-06-27 10:38:08.000000000 +0800
-@@ -36,7 +36,7 @@ struct oprofile_cpu_buffer {
+               add_code(cpu_buf, (unsigned long)task);
+       }
+@@ -269,6 +275,25 @@
+       add_sample(cpu_buf, pc, 0);
+ }
+ 
++int oprofile_add_domain_switch(int32_t domain_id)
++{
++      struct oprofile_cpu_buffer * cpu_buf = &cpu_buffer[smp_processor_id()];
++
++      /* should have space for switching into and out of domain 
++         (2 slots each) plus one sample and one cpu mode switch */
++      if (((nr_available_slots(cpu_buf) < 6) && 
++           (domain_id != COORDINATOR_DOMAIN)) ||
++          (nr_available_slots(cpu_buf) < 2))
++              return 0;
++
++      add_code(cpu_buf, CPU_DOMAIN_SWITCH);
++      add_sample(cpu_buf, domain_id, 0);
++
++      current_domain = domain_id;
++
++      return 1;
++}
++
+ /*
+  * This serves to avoid cpu buffer overflow, and makes sure
+  * the task mortuary progresses
+diff -Naur orig/drivers/oprofile/cpu_buffer.h new/drivers/oprofile/cpu_buffer.h
+--- orig/drivers/oprofile/cpu_buffer.h 2006-05-02 14:38:44.000000000 -0700
++++ new/drivers/oprofile/cpu_buffer.h  2006-07-06 18:19:05.000000000 -0700
+@@ -36,7 +36,7 @@
        volatile unsigned long tail_pos;
        unsigned long buffer_size;
        struct task_struct * last_task;
@@ -221,7 +244,7 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/cpu_buffer.h ./drivers/op
        int tracing;
        struct op_sample * buffer;
        unsigned long sample_received;
-@@ -51,7 +51,13 @@ extern struct oprofile_cpu_buffer cpu_bu
+@@ -51,7 +51,10 @@
  void cpu_buffer_reset(struct oprofile_cpu_buffer * cpu_buf);
  
  /* transient events for the CPU buffer -> event buffer */
@@ -230,17 +253,14 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/cpu_buffer.h ./drivers/op
 +#define CPU_MODE_USER           0
 +#define CPU_MODE_KERNEL         1
 +#define CPU_MODE_XEN            2
-+#define CPU_MODE_PASSIVE_START  3
-+#define CPU_MODE_PASSIVE_STOP   4
-+#define CPU_TRACE_BEGIN         5
-+
-+#define IGNORED_PC              0
++#define CPU_TRACE_BEGIN         3
++#define CPU_DOMAIN_SWITCH       4
  
  #endif /* OPROFILE_CPU_BUFFER_H */
-diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/event_buffer.h ./drivers/oprofile/event_buffer.h
---- ../pristine-linux-2.6.16.13/drivers/oprofile/event_buffer.h        2006-05-03 05:38:44.000000000 +0800
-+++ ./drivers/oprofile/event_buffer.h  2006-06-19 22:43:53.000000000 +0800
-@@ -29,11 +29,14 @@ void wake_up_buffer_waiter(void);
+diff -Naur orig/drivers/oprofile/event_buffer.h new/drivers/oprofile/event_buffer.h
+--- orig/drivers/oprofile/event_buffer.h       2006-05-02 14:38:44.000000000 -0700
++++ new/drivers/oprofile/event_buffer.h        2006-07-06 18:19:05.000000000 -0700
+@@ -29,15 +29,20 @@
  #define CPU_SWITCH_CODE               2
  #define COOKIE_SWITCH_CODE            3
  #define KERNEL_ENTER_SWITCH_CODE      4
@@ -251,14 +271,20 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/event_buffer.h ./drivers/
  #define TRACE_BEGIN_CODE              8
  #define TRACE_END_CODE                        9
 +#define XEN_ENTER_SWITCH_CODE         10
-+#define PASSIVE_START_CODE            11
-+#define PASSIVE_STOP_CODE             12
++#define DOMAIN_SWITCH_CODE            11
   
  #define INVALID_COOKIE ~0UL
  #define NO_COOKIE 0UL
-diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/oprof.c ./drivers/oprofile/oprof.c
---- ../pristine-linux-2.6.16.13/drivers/oprofile/oprof.c       2006-05-03 05:38:44.000000000 +0800
-+++ ./drivers/oprofile/oprof.c 2006-06-19 23:45:17.000000000 +0800
+ 
++/* Constant used to refer to coordinator domain (Xen) */
++#define COORDINATOR_DOMAIN -1
++
+ /* add data to the event buffer */
+ void add_event_entry(unsigned long data);
+  
+diff -Naur orig/drivers/oprofile/oprof.c new/drivers/oprofile/oprof.c
+--- orig/drivers/oprofile/oprof.c      2006-05-02 14:38:44.000000000 -0700
++++ new/drivers/oprofile/oprof.c       2006-07-06 18:19:05.000000000 -0700
 @@ -5,6 +5,10 @@
   * @remark Read the file COPYING
   *
@@ -279,7 +305,7 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/oprof.c ./drivers/oprofil
  struct oprofile_operations oprofile_ops;
  
  unsigned long oprofile_started;
-@@ -33,6 +37,32 @@ static DECLARE_MUTEX(start_sem);
+@@ -33,6 +37,32 @@
   */
  static int timer = 0;
  
@@ -312,10 +338,10 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/oprof.c ./drivers/oprofil
  int oprofile_setup(void)
  {
        int err;
-diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/oprof.h ./drivers/oprofile/oprof.h
---- ../pristine-linux-2.6.16.13/drivers/oprofile/oprof.h       2006-05-03 05:38:44.000000000 +0800
-+++ ./drivers/oprofile/oprof.h 2006-06-19 23:42:36.000000000 +0800
-@@ -35,5 +35,8 @@ void oprofile_create_files(struct super_
+diff -Naur orig/drivers/oprofile/oprof.h new/drivers/oprofile/oprof.h
+--- orig/drivers/oprofile/oprof.h      2006-05-02 14:38:44.000000000 -0700
++++ new/drivers/oprofile/oprof.h       2006-07-06 18:19:05.000000000 -0700
+@@ -35,5 +35,8 @@
  void oprofile_timer_init(struct oprofile_operations * ops);
  
  int oprofile_set_backtrace(unsigned long depth);
@@ -324,9 +350,9 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/oprof.h ./drivers/oprofil
 +int oprofile_set_passive(int passive_domains[], unsigned int pdomains);
   
  #endif /* OPROF_H */
-diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/oprofile_files.c ./drivers/oprofile/oprofile_files.c
---- ../pristine-linux-2.6.16.13/drivers/oprofile/oprofile_files.c      2006-05-03 05:38:44.000000000 +0800
-+++ ./drivers/oprofile/oprofile_files.c        2006-06-19 23:29:07.000000000 +0800
+diff -Naur orig/drivers/oprofile/oprofile_files.c new/drivers/oprofile/oprofile_files.c
+--- orig/drivers/oprofile/oprofile_files.c     2006-05-02 14:38:44.000000000 -0700
++++ new/drivers/oprofile/oprofile_files.c      2006-07-06 18:19:05.000000000 -0700
 @@ -5,15 +5,21 @@
   * @remark Read the file COPYING
   *
@@ -350,7 +376,7 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/oprofile_files.c ./driver
  unsigned long fs_buffer_size = 131072;
  unsigned long fs_cpu_buffer_size = 8192;
  unsigned long fs_buffer_watershed = 32768; /* FIXME: tune */
-@@ -117,11 +123,202 @@ static ssize_t dump_write(struct file * 
+@@ -117,11 +123,202 @@
  static struct file_operations dump_fops = {
        .write          = dump_write,
  };
@@ -554,8 +580,9 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/oprofile_files.c ./driver
        oprofilefs_create_file(sb, root, "buffer", &event_buffer_fops);
        oprofilefs_create_ulong(sb, root, "buffer_size", &fs_buffer_size);
        oprofilefs_create_ulong(sb, root, "buffer_watershed", &fs_buffer_watershed);
---- ../pristine-linux-2.6.16.13/include/linux/oprofile.h       2006-05-03 05:38:44.000000000 +0800
-+++ ./include/linux/oprofile.h 2006-06-19 23:52:00.000000000 +0800
+diff -Naur orig/include/linux/oprofile.h new/include/linux/oprofile.h
+--- orig/include/linux/oprofile.h      2006-05-02 14:38:44.000000000 -0700
++++ new/include/linux/oprofile.h       2006-07-06 18:19:31.000000000 -0700
 @@ -16,6 +16,8 @@
  #include <linux/types.h>
  #include <linux/spinlock.h>
@@ -565,7 +592,7 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/oprofile_files.c ./driver
   
  struct super_block;
  struct dentry;
-@@ -27,6 +29,11 @@ struct oprofile_operations {
+@@ -27,6 +29,11 @@
        /* create any necessary configuration files in the oprofile fs.
         * Optional. */
        int (*create_files)(struct super_block * sb, struct dentry * root);
@@ -577,3 +604,12 @@ diff -pru ../pristine-linux-2.6.16.13/drivers/oprofile/oprofile_files.c ./driver
        /* Do any necessary interrupt setup. Optional. */
        int (*setup)(void);
        /* Do any necessary interrupt shutdown. Optional. */
+@@ -68,6 +75,8 @@
+ /* add a backtrace entry, to be called from the ->backtrace callback */
+ void oprofile_add_trace(unsigned long eip);
+ 
++/* add a domain switch entry */
++int oprofile_add_domain_switch(int32_t domain_id);
+ 
+ /**
+  * Create a file of the given name as a child of the given root, with